Adding support for semantic color for icons #1182
Conversation
LFDanLu
requested review from
dannify,
devongovett and
snowystinger
as code owners
|
Build successful! 🎉 |
LFDanLu
changed the title
|
Build successful! 🎉 |
|
Build successful! 🎉 |
| @@ -10,6 +10,8 @@ | |||
| * governing permissions and limitations under the License. | |||
| */ | |||
|
|
|||
| // This file is generated by lib/varsToTypeScript.js! DO NOT EDIT. | |||
There was a problem hiding this comment.
generated by running varsToTypeScript, not entirely sure if we wanna keep or not.
There was a problem hiding this comment.
but why wasn't it there previously?
There was a problem hiding this comment.
dunno, removed in this pull: #206. Not sure the reason for its removal, seems innocuous to me (helps others understand where the file came from)
There was a problem hiding this comment.
yeah, i'm definitely in favor of leaving it in if it is truly a generated file
| ```tsx example | ||
| import Alert from '@spectrum-icons/workflow/Alert'; | ||
|
|
||
| <Alert /> |
There was a problem hiding this comment.
Should we wrap these in a Flex with a gap so they aren't quite that close together?
Also we should put an aria-label on them, otherwise we'll likely get some warnings
| @@ -95,6 +99,8 @@ export type DimensionValue = | |||
| | number; | |||
|
|
|||
| export type ColorValue = | |||
| | 'status' | |||
| | 'version' | |||
There was a problem hiding this comment.
These seem like parsing issues. I don't think they are real variable names?
There was a problem hiding this comment.
ah you are right, I'll update the regex in the script
|
Build successful! 🎉 |
Closes #1159
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: